Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor of docker PR #11405 #336

Merged
merged 1 commit into from
Jul 9, 2015
Merged

Conversation

alecbenson
Copy link
Contributor

This is a port of docker PR #11405 code into the libnetwork codebase.
It will solve docker issue #11404.

Signed-off-by: Alec Benson albenson@redhat.com

//Gets the IP version in use ( [ipv4], [ipv6] or [ipv4 and ipv6] )
func getIPVersion(config *networkConfiguration) ipVersion {
ipVersion := ipv4
if config.FixedCIDR != nil || config.EnableIPv6 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be FixedCIDRv6 ?

@alecbenson alecbenson force-pushed the check-kernel-params branch from 68d1fc4 to 842f2e3 Compare June 26, 2015 14:36
@alecbenson
Copy link
Contributor Author

@aboch Thanks for looking this over. I just made both of the changes that you suggested.

@alecbenson alecbenson force-pushed the check-kernel-params branch from 842f2e3 to 11a8885 Compare June 26, 2015 14:48
@aboch
Copy link
Contributor

aboch commented Jun 26, 2015

I just realized in docker/docker PR #11405 @minimar also added some test code. Could you please port that one as well, so we do not leave anything out. Thanks.

@alecbenson alecbenson force-pushed the check-kernel-params branch from 11a8885 to addb20d Compare June 26, 2015 16:15
@alecbenson
Copy link
Contributor Author

Sure -- the test has been included now.

@aboch
Copy link
Contributor

aboch commented Jun 26, 2015

Thanks for porting the patch to libnetwork.

LGTM

@alecbenson
Copy link
Contributor Author

Any chance somebody could merge this in?

@dave-tucker
Copy link
Contributor

👍 LGTM
/cc @mrjana @mavenugo

if err != nil {
if ptherr, ok := err.(*os.PathError); ok {
if errno, ok := ptherr.Err.(syscall.Errno); ok && errno == syscall.ENOENT {
if netutils.IsRunningInContainer() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IsRunningInContainer cannot be reliably used outside libnetwork test framework. This needs special environment variables to be set up and this is not going to work when libnetwork is used in docker. So this part of the code needs to be redesigned so that this function works everywhere.

@alecbenson alecbenson force-pushed the check-kernel-params branch 2 times, most recently from f23b441 to 0e373fc Compare July 8, 2015 14:05
Signed-off-by: Alec Benson <albenson@redhat.com>
@alecbenson alecbenson force-pushed the check-kernel-params branch from 0e373fc to ec88bd0 Compare July 8, 2015 14:15
@alecbenson
Copy link
Contributor Author

@mrjana
There we go. That should be taken care of now.

@mrjana
Copy link
Contributor

mrjana commented Jul 9, 2015

Thanks @alecbenson. LGTM now

mrjana added a commit that referenced this pull request Jul 9, 2015
@mrjana mrjana merged commit 2d09474 into moby:master Jul 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants